Skip to content

docs(hpc/soa): P2 savant tightenings — f32-only scope + hpc::soa layering rationale + integration test ungate (orphan rescue from #156)#157

Merged
AdaWorldAPI merged 1 commit into
masterfrom
claude/w3-w6-p2-savant-followup
May 18, 2026
Merged

docs(hpc/soa): P2 savant tightenings — f32-only scope + hpc::soa layering rationale + integration test ungate (orphan rescue from #156)#157
AdaWorldAPI merged 1 commit into
masterfrom
claude/w3-w6-p2-savant-followup

Conversation

@AdaWorldAPI
Copy link
Copy Markdown
Owner

Summary

Three small follow-ups from the P2 codex savant review of the just-merged PR #156 (W3-W6 SoA/AoS helpers). The review's verdict was SHIP-WITH-FOLLOWUPS — #156 merged before this commit could land on its branch, so the follow-ups carry as their own PR.

The full review doc is included: .claude/knowledge/w3-w6-p2-savant-review.md — covers six review categories (A. ergonomics, B. naming, C. doc-prose, D. distance-typing visibility, E. future-proofing, F. CI signal) and the net call.

What's in

C3 + A4 (src/hpc/soa.rs module header expansion)

Two related doc gaps the P2 savant flagged:

  • A4 — f32-only scope undocumented. aos_to_soa<T, N, F: Fn(&T) -> [f32; N]> and soa_to_aos are hardwired to SoaVec<f32, N>. Downstream consumers with i8/u8/u16/bf16 SoA fields (palette indices, quantized embeddings, BF16 mantissa bytes) cannot use the public helpers today; module header now says this explicitly. The macro itself supports any field type.
  • C3 — hpc::soa vs simd_ops placement rationale. Module header explains why these helpers live at the hpc:: level and not in crate::simd_ops: the W1a consumer contract litmus rejects generic-library free functions from the SIMD-dispatch glue layer. Saves a future contributor from re-deriving the rule.

A3 (src/hpc/bulk.rs integration test ungate)

The bulk_apply × aos_to_soa integration test was gated behind #[cfg(any())] (canonical never-compile sentinel) by Worker B because hpc::soa wasn't yet on the branch base when the worker ran. Now that both modules co-merged in #156, the deferral reason is gone. Ungating exercises the canonical SoA-stage-then-process compose pattern: outer bulk_apply chunk loop, inner aos_to_soa per-chunk staging, closure body processes the SoA layout.

hpc::bulk test count: 16 → 17.

P2 savant review doc

.claude/knowledge/w3-w6-p2-savant-review.md — the savant's full advisory review of PR #156, persisted for future sessions.

What's NOT in (P2 follow-ups deferred to a separate cleanup PR)

The savant flagged additional P2s. Deferred to a future cleanup PR:

  • A1soa_struct! macro is not parity with the existing GaussianBatch in splat3d/gaussian.rs. Missing: lane-padded with_capacity (the pad_to_lanes(n, PREFERRED_F32_LANES) discipline), reserve, truncate, swap_remove, iter_mut. Means the macro is the "simple SoA struct" path; GaussianBatch stays bespoke until the macro grows a with_capacity_padded knob.
  • A2#[inline] hints on aos_to_soa / soa_to_aos / bulk_apply / bulk_scan. Useful for hot-path inlining of the user closure across crate boundaries.
  • B3bulk_scan naming. Rust idiom Iterator::scan = fold-with-state; this fn is a chunks walker. Rename to bulk_for_each / bulk_view before any external consumer adopts the name.
  • C1 — Realistic SoA-stage-then-SIMD doctests on SoaVec::chunks and aos_to_soa (current doctests are 2-element smoke tests).
  • C2 — Per-fn docstrings consistently note "scalar today" (currently mixed messaging across the two files).
  • E1 — Extract aos_to_soa_scalar / soa_to_aos_scalar private fns so the future SIMD swap is a clean arm-addition not a structural refactor.

Test plan


Generated by Claude Code

…ring rationale + integration test ungate

Three small follow-ups from the P2 codex savant review of merged
PR #156 (review doc at .claude/knowledge/w3-w6-p2-savant-review.md
on the prior branch, also persists the rationale).

C3 + A4 (src/hpc/soa.rs module header):
- Document that aos_to_soa / soa_to_aos are currently hardwired to
  f32 output. i8 / u8 / u16 / bf16 SoA consumers must hand-roll the
  extract loop today; macro itself supports any field type.
- Explain WHY these helpers live in hpc::soa and not crate::simd_ops:
  W1a consumer contract litmus rejects 'generic-library free
  functions' from the SIMD-dispatch glue layer; pure-scalar layout
  helpers belong at hpc:: co-located with the data type.

A3 (src/hpc/bulk.rs): ungate the bulk_apply x aos_to_soa integration
test. Worker-isolation deferral reason is gone now that both modules
co-merged in #156. Test exercises the canonical SoA-stage-then-process
compose pattern. bulk test count 16 -> 17.

Non-blocking from savant view; ship-with-followups verdict. Other
deferred P2s (soa_struct! parity with GaussianBatch lane-padded
with_capacity, bulk_scan rename, #[inline] hints on hot-path entries,
realistic SIMD-staging doctests) are queued for a separate cleanup PR.
@AdaWorldAPI AdaWorldAPI merged commit c1228ae into master May 18, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants